Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FP fix: Update link_credential_phishing_intent_and_other_indicators.yml #790

Merged
merged 2 commits into from
Sep 18, 2023
Merged

FP fix: Update link_credential_phishing_intent_and_other_indicators.yml #790

merged 2 commits into from
Sep 18, 2023
+4 −3

Conversation

morriscode
Copy link
Member

Falsely returns true if recipient is null. Adding a valid check

@morriscode
Update link_credential_phishing_intent_and_other_indicators.yml
c77b714 
Falsely returns true if recipient is null. Adding a valid check
@morriscode morriscode requested a review from a team September 13, 2023 22:09
@morriscode
Copy link
Member Author

Just wanted to highlight the 5 or so regressions caused here.
Several of them are too old and have current_thread and the rule has since been changed to use that.
The other failures are due to the bug of us matching on recipient found in the url, even though the recipient was null.

Could add a null recipient check as an or condition but that reintroduces the problem

( length(recipients.to) == 0 or all(recipients.to, .display_name == "Undisclosed recipients") ) and length(recipients.cc) == 0 and length(recipients.bcc) == 0 ), ) )

@morriscode morriscode enabled auto-merge (squash) September 18, 2023 13:30
@morriscode morriscode merged commit 8a98a85 into main Sep 18, 2023
3 of 4 checks passed
@morriscode morriscode deleted the sam.fp.credphish.otherindicators branch September 18, 2023 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkamdjou jkamdjou jkamdjou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@morriscode @jkamdjou